-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
list machine objects #367
list machine objects #367
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- it's not necessary to hold additional info on the options struct - get rid of it and move the code in the complete function to the place where the machines are fetched
- fetch the nodes and just try to fetch the machines (ignore forbidden errors and just log all other erros like it was done in the other PR) and pass both lists to the NewConnectInformation and handle it there instead of creating fake corev1.Node structs
…ll not be displayed.
aced38e
to
043d835
Compare
Hi @tedteng, thank you for your effort on the pull request. I see that it would require some extensive revisions. To maintain efficiency and to help guide the process, I've initiated a new PR (#368) that incorporates some of your ideas. I suggest we focus our efforts there. You may want to close this one. Please don't hesitate to review the new PR and let me know your thoughts. Your contributions are valuable and appreciated. |
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #361
Special notes for your reviewer:
Release note: